Skip to content

Conversation

@jbazkar
Copy link

@jbazkar jbazkar commented Nov 22, 2025

Problem

When an AWS::SSO::PermissionSet contains an inline policy, cdk diff renders IAM Statement Changes with an empty Principal column. This makes the diff difficult to interpret, especially when multiple Permission Sets exist in the same stack, because the IAM Statement table does not indicate which Permission Set the statement belongs to.

This issue is reported in #956.

Solution

This PR introduces a minimal and targeted enhancement to the diff engine:
When the inline policy comes from an AWS::SSO::PermissionSet, and the derived principal would otherwise be empty:
The diff engine now assigns a pseudo-principal:

AWS:${<LogicalId>}

This keeps the diff output consistent and readable, letting developers immediately see which Permission Set owns each IAM statement.

All existing behavior for IAM Roles, Users, Policies, Groups, and non-PermissionSet resources is unchanged.

Implementation Details

  • Added readPermissionSetInlinePolicy, which:
    -> Parses inline statements,
    -> Applies defaultResource (existing behavior),
    -> Applies defaultPrincipal with a PermissionSet-specific pseudo-principal.
  • Updated the InlineResourcePolicy branch in readPropertyChange to route PermissionSet policies through this new helper.
  • No changes were made to synthesis or deployment logic.

Tests

Updated PermissionSet-related expectations in
test/iam/detect-changes.test.ts to reflect the new principal.

All IAM change detection tests now validate the presence of the pseudo-principal.

Full test suite executed successfully:
This change affects only the human-readable diff output.
Full test suite executed successfully:

Test Output (All Green)

All suites and tests passed locally after the PermissionSet principal enhancement:
Here is the full output of yarn test from my local environment:
tests-passed

Fixes #956


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.72%. Comparing base (2e92d71) to head (7d79b5c).
⚠️ Report is 21 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #953   +/-   ##
=======================================
  Coverage   87.72%   87.72%           
=======================================
  Files          72       72           
  Lines       10087    10087           
  Branches     1330     1330           
=======================================
  Hits         8849     8849           
  Misses       1213     1213           
  Partials       25       25           
Flag Coverage Δ
suite.unit 87.72% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(cdk): Inconsistent rendering of IAM Statement Changes for PermissionSets

4 participants